-
Notifications
You must be signed in to change notification settings - Fork 773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TIND ILS: Add 269, 720 fields #3413
base: master
Are you sure you want to change the base?
Conversation
#3409 is merged, so go ahead and rebase! |
Will do, out sick for the moment so it might be a bit 😄 |
- 720 fields are sometimes used e.g. in datasets to describe additional contributors. - Use 720 field as a contributor. - Note that tests will fail until zotero#3409 is merged. This is because the MARC code assumes that all books with no authors that have contributors are actually meant to be editors, which is invalid. However, we have no way of setting items to be not Book without zotero#3409.
- Refactor MARCXML importer to expose a utility function, similar to the MARC importer. This will allow consumers of the MARCXML translator to instead decide to manually drive the process.
- Convert to using the MARCXML utility function directly - Use the MARCXML utility to read date from the 269 field, which is a non-standard MARC field.
5a4a049
to
257dfde
Compare
I have updated the branch. It seems CI fails because I modified MARC.js and MARCXML.js which are used by over 10 translators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This generally looks great. Some comments/suggestions below.
MARC.js
Outdated
"creators": [ | ||
{ | ||
"lastName": "Meta Platforms, Inc", | ||
"creatorType": "contributor", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is failing because the translator returns editor
, not contributor
. I think editor
is more correct, but the test needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, editor is what would be expected:
- since the example MARC doesn't have a leader, it defaults to book
- The MARC doesn't have any authors
- There's some code in MARC.js that sets all creators to editor if a book has no authors
I don't know if more correct is what I'd say in terms of how MARC should behave, though.
This PR will remain a draft until #3409 is merged for two reasons: